feat: HTML/CSS/JS ContextWriter #636

alehechka · 2024-03-21T19:53:32Z

Overview

This is another step toward HTML LSP support in templ, related to the discussion in #498. This PR specifically implements the ContextWriter that @a-h proposed in this comment.

With these changes, I updated nearly all the unit tests to include an expected output format for each context where appropriate (i.e. JS for JS, HTML for HTML). We could add expected output for each individual context, but I skipped on that for now since they would all end up being a huge series of whitespace characters.

a-h

Great progress on this. Thanks for working on it.

I think we need to keep the text length exactly the same, so that we can apply changes made by the LSP.

However, some of these changes adjust the size of content. I think it would be worth adding an assertion for each test that:

The number of lines in the output is the same as the input.
That each line is the same length as the input.

That could be modelled by:

func getLineLengths(s string) (lengths []int) {
  lines := strings.Split(s, "\n")
  lengths = make([]int, len(lines))
  for i := 0; i < len(lines); i ++ {
    lengths[i] = len(lines[i])
  }
  return lengths
}

Then, compare the arrays with cmp.Diff or something.

a-h · 2024-03-29T15:44:22Z

parser/v2/cssparser_test.go

-			input: "background-color:\r\n{ constants.BackgroundColor };\r\n",
+			name:        "css: single constant property with windows newlines",
+			input:       "background-color:\r\n{ constants.BackgroundColor };\r\n",
+			expectedCSS: "background-color: '                           ';\n",


For target LSPs to work well, the character positions can't change in the file.

Our goal is to be able to send the current file position (line, col) to the LSP and have it send text insertions and other things back to templ. When we receive those changes, ideally, I'd want to be able to put them into the file without having to remap their positions.

However, this change removes the \r\n and \r chars, therefore changing the line pos.

Here, we're losing \r\n, so the line position

The expected CSS is presumably the output after parsing, which is essentially the same as what we get if we've formatted the code.

However, in our case, we want to support the user continuously typing into the editor, before they've formatted the code, so I'm not sure this is the right behaviour. Maybe it is, and I'm not understanding the context of the test though.

The expected CSS is presumably the output after parsing, which is essentially the same as what we get if we've formatted the code.

I believe this to be the behavior that is causing all of these length mismatches. Since the entire document is parsed into a series of nodes and then built back up with the formatting specified in all of the Write() methods, this is the default output after the changes I've made with the ContextWriter.

I added the suggested line length checking to all of the unit tests that I updated. There were quite a few mismatches outside of the ones you caught so those are failing as of 4c7959b.

I've started on a solution in another branch on my fork: alehechka/templ@feature/html-lsp...alehechka:templ:fix/mismatch-line-lengths.

The rough idea is to include a perservePositions bool on the ContextWriter that will determine how certain nodes get formatted. As of this comment, this is resolving the manipulation of self-closing tags. The next step is reconciling extraneous white space between the source and output.

a-h · 2024-03-29T15:46:47Z

parser/v2/cssparser_test.go

@@ -180,6 +220,9 @@ func TestCSSParser(t *testing.T) {
 			input: `css Name() {
 background-color: #ffffff;
 }`,
+			expectedCSS: `            
+	background-color: #ffffff;


Similarly, here, we've got a bit of extra spacing.

a-h · 2024-03-29T15:46:56Z

parser/v2/cssparser_test.go

@@ -210,6 +253,9 @@ background-color: #ffffff;
 			input: `css Name() {
 background-color: { constants.BackgroundColor };
 }`,
+			expectedCSS: `            
+	background-color: '                           ';


And here too.

a-h · 2024-03-29T15:50:07Z

parser/v2/cssparser_test.go

@@ -256,6 +302,9 @@ background-color: { constants.BackgroundColor };
 			input: `css Name(prop string) {
 background-color: { prop };
 }`,
+			expectedCSS: `                       
+	background-color: '      ';


More additional space.

a-h · 2024-03-29T15:50:54Z